Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| /// | ||
| /// Prefer named helpers (e.g. [`AppError::not_found`]) where it clarifies | ||
| /// intent. | ||
| pub fn with(kind: AppErrorKind, msg: impl Into<String>) -> Self { | ||
| pub fn with(kind: AppErrorKind, msg: impl Into<Cow<'static, str>>) -> Self { | ||
| Self { |
There was a problem hiding this comment.
[P1] Restrict AppError constructors to 'static string slices
Changing the message parameter to impl Into<Cow<'static, str>> means only 'static borrows are accepted. Callers that previously passed short‑lived &str values (e.g. AppError::bad_request(err.as_str())) now hit a compile error because such slices no longer implement Into<Cow<'static, str>>. This contradicts the stated goal of accepting borrowed or owned strings and introduces a breaking API change unless every non‑static borrow is converted to String first. Consider accepting impl Into<Cow<'_, str>> and promoting non‑'static values to owned strings internally, or keep the original impl Into<String> and wrap with Cow::Owned so existing call sites continue to compile.
Useful? React with 👍 / 👎.
Summary
Cow<'static, str>and accept borrowed or owned stringsAppErrorintoErrorResponsewithout extra allocationsTesting
cargo clippy --all-targets -- -D warningscargo build --all-targetscargo test --allcargo doc --no-depshttps://chatgpt.com/codex/tasks/task_e_68c275eacc48832bbf2e2fbffb2de46d